Skip to content

Extract terminal file-upload logic into terminal-uploads.ts#796

Merged
selfcontained merged 1 commit into
mainfrom
tech-debt/extract-terminal-uploads
Jul 18, 2026
Merged

Extract terminal file-upload logic into terminal-uploads.ts#796
selfcontained merged 1 commit into
mainfrom
tech-debt/extract-terminal-uploads

Conversation

@selfcontained

Copy link
Copy Markdown
Owner

What

Moves the self-contained file-upload orchestration out of the 916-line use-terminal.ts hook into a new apps/web/src/hooks/terminal-uploads.ts module. The hook's uploadFiles callback now delegates to uploadTerminalFiles(agentId, files, setUploadingFiles).

Why it's tech debt

use-terminal.ts is a flagged complexity hotspot. The drag/drop + clipboard-paste upload logic (file-type validation, the upload loop, and toast feedback) had no dependency on the terminal connection lifecycle — it only needed the connected agent id and the "uploading" state setter. Pulling it into a module continues the established extraction pattern already applied to this file (terminal-socket.ts #659, terminal-surface.ts #664) and keeps the hook body focused on the WebSocket/connection lifecycle.

This is a pure behavior-preserving code move — the extracted function is byte-identical logic, and the hook sheds its sonner and media-upload imports. Hook shrinks 916 → 867 lines.

Not in scope

The backlog item's original framing ("split use-terminal.ts into focused connection/resize/reconnect/focus hooks") was deliberately not attempted — that logic is tightly coupled through ~15 shared refs and the subtle WebSocket reconnect lifecycle, so a hook split would be high-risk churn with no behavior change. This PR takes the safe, incremental slice instead. Pre-existing issues in the file are untouched.

Validation

  • pnpm run check
  • pnpm run finalize:web
  • pnpm run test:e2e ✓ (168 passed, 13 terminal-live tests skipped as usual — they require a live tmux env)

Queued for next run

Re-audit remaining complexity hotspots; agents-view.tsx (1006 lines) is on the backlog but is a hot, frequently-changed file — verify no in-flight work before touching it.

🤖 Generated with Claude Code

use-terminal.ts is a 916-line complexity hotspot. The self-contained
drag/drop + paste upload orchestration (validation, upload loop, toast
feedback) had no dependency on the terminal connection lifecycle, so it
moves cleanly into a new terminal-uploads.ts module — continuing the
established extraction pattern (terminal-socket.ts #659, terminal-surface.ts
#664). Pure code move: the hook's uploadFiles callback now delegates to
uploadTerminalFiles(). No behavior change.

Hook shrinks 916 -> 867 lines and sheds the sonner/media-upload imports.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@selfcontained
selfcontained merged commit caa9148 into main Jul 18, 2026
1 check passed
@selfcontained
selfcontained deleted the tech-debt/extract-terminal-uploads branch July 18, 2026 09:11
selfcontained added a commit that referenced this pull request Jul 20, 2026
)

The E2E harness ran its server via `pnpm run dev`, which is
`bun --watch src/main.ts`. `prepare:runtime-assets` — run at the start of
the server's `test`, `check`, `build`, and `start` scripts — unconditionally
rewrites `apps/server/src/generated/runtime-assets.js`, which lives inside
that watched tree. So any concurrent pnpm task restarted the server mid-run
and dropped whatever request was in flight, surfacing as `socket hang up`
and aborting the rest of the suite.

Switch the webServer command to `pnpm run start`, which is the same boot
path without the watcher. E2E never needs hot reload, and the asset write
inside `start` happens before the server binds, so it cannot self-restart.

A/B on e2e/worktree.spec.ts under concurrent asset regeneration:
  before  6 failed / 18 passed (51.6s)
  after            24 passed (10.3s)

This also explains the long-standing "app shell fails to become visible
under parallel load" flake: a restart during a page reload stalls that page
while unrelated tests keep passing. Re-enable the split-pane test that was
skipped for it — 30/30 under --repeat-each=5 with the same write pressure,
and the full suite is green under it too.

Add unit coverage for uploadTerminalFiles, which had none since it was
extracted in #796: guard clauses, supported/unsupported partitioning and
message pluralization, the uploading flag lifecycle, and per-file failure
aggregation. Verified non-vacuous — six source mutations each fail at
least one test.

Co-authored-by: Claude Opus 4.8 (1M context) <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant